W-21354759: Implement SF Payments support in create app package#3698
W-21354759: Implement SF Payments support in create app package#3698amittapalli merged 7 commits intot/team404/sfp-on-pwafrom
Conversation
…ommerceCloud/pwa-kit into amittapalli/pwa-smoke-testing merge changes:
…elds, and revert use current basket hook changes
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
| // Example URLs: sdkUrl: 'https://<instance>.unified.demandware.net/on/demandware.static/Sites-Site/-/-/internal/jscript/sfp/v1/sfp.js' | ||
| // metadataUrl: 'https://<instance>.unified.demandware.net/on/demandware.static/Sites-Site/-/-/internal/metadata/v1.json' | ||
| sfPayments: { | ||
| enabled: true, |
There was a problem hiding this comment.
Should our default be true or false? If false then generated sites could be used for demo purposes with placeholder buttons and fake payments. If true then they could only check out if they set up a merchant account and so on for SFP in BM.
There was a problem hiding this comment.
So, its a combination of value from this switch and what is coming from shopper config API.
If both true -> feature enabled
If api true but customer decides to switch to false themselves (then its disabled)
If api false but customer kept as true (then still disabled since instance does not allow)
If api false and customer also changed to false (then its obviously disabled)
That way if there are customers that have already customized some of their components, they may not be stuck with a broken experience since they have a kill switch at all times.
We can raise this to Product as well to confirm
There was a problem hiding this comment.
Do we even need this flag then? I seem to remember introducing it because we didn't have the shopper config API changes yet.
There was a problem hiding this comment.
Helps to keep it since it works as the real kill switch for customers who end up with a mixed mode of behavior. In the past we had this flag but it was never really applied or used. With this PR, I am trying to make use of it. All new customers will have SF Payments enabled regardless. But as I mentioned it allows a customer (especially existing ones) to disable it if they face any unforseen issues when the FT is true
| '*.paypal.com', | ||
| '*.adyen.com', | ||
| '*.google.com', | ||
| '*.demandware.net', // Used to load a valid payment scripts in test environment |
There was a problem hiding this comment.
Let's leave this demandware.net one out of the generated template.
There was a problem hiding this comment.
I can remove it though I noticed that PWA team always had it for the img src in their original code. So
'img-src': [
// Default source for product images - replace with your CDN
'.commercecloud.salesforce.com',
'.demandware.net'
],
| '*.paypal.com', | ||
| '*.adyen.com', | ||
| '*.google.com', | ||
| '*.demandware.net', // Used to load a valid payment scripts in test environment |
There was a problem hiding this comment.
Same as above. Any idea why there are two copies of this?
There was a problem hiding this comment.
Ah. One is for customers who choose to not extend but get a copy of the template while the other is for customers who don't want the template but extend from the template. So both copies of handle bars are needed
| countryCode === 'CA' | ||
| ? 'Please select your province.' // FYI we won't translate this | ||
| : formatMessage({ | ||
| ? 'Please select your province.' |
There was a problem hiding this comment.
Should we keep that comment about not translating it?
| parameters: { | ||
| currency: paymentCurrency, | ||
| countryCode: paymentCountryCode || fallbackCountryCode || 'US' // TODO: remove US when parameter made optional | ||
| countryCode: paymentCountryCode || fallbackCountryCode || 'US' |
There was a problem hiding this comment.
If this parameter is now optional for the payment config API I'd prefer to remove the fallback. There seem to be other PWA hardcoded US and CA values but we don't have to force an optional parameter to have a value.
There was a problem hiding this comment.
country code is required apparently and currency is optional. So felt its safer to leave the fallback though with the updated logic, we may never need this fallback but kept it just incase
|
|
||
| /** | ||
| * Custom hook to check if Salesforce Payments is enabled | ||
| * //?? true means: if the config is missing, default to "don't block it" |
There was a problem hiding this comment.
nit: remove the "//" in the comment unless I'm missing something
| "push": "npm run build && pwa-kit-dev push", | ||
| "save-credentials": "pwa-kit-dev save-credentials", | ||
| "start": "cross-env NODE_ICU_DATA=node_modules/full-icu pwa-kit-dev start", | ||
| "start:https": "cross-env NODE_TLS_REJECT_UNAUTHORIZED=0 NODE_ICU_DATA=node_modules/full-icu DEV_SERVER_PROTOCOL=https DEV_SERVER_SSL_FILE_PATH=../../localhost.pem pwa-kit-dev start", |
There was a problem hiding this comment.
ah yes. We used it for localhost testing since we can't do https locally with the pwa code. But planning on removing these before the final merge
Multiple issues have been addressed with this PR. Please see the details in the description below
Description
useAddressFields.jsx — internationalized stateCode/postalCode fields
Without this change (original code), if a customer adds {value: 'DE', label: 'Germany'} to SHIPPING_COUNTRY_CODES:
stateCode field: type is always 'select', options is always countryCode === 'CA' ? provinceOptions : stateOptions. So Germany gets a dropdown with US states (Alabama, Alaska...). So, its already broken!
stateCode required: Always required. So a German customer is forced to pick a US state. Completely wrong.
postalCode label: Says "Zip Code" for everything except CA. Minor but incorrect.
I feel the entire address form is wrong and needs rework but we don't have time for that. So I am trying to introduce the least invasive risk. Existing customers (US or Canada) will not see any difference but other international customers will see a textbox for state/province instead of a hard-coded US dropdown.
hooks/use-sf-payments-country.js -- dynamic country for payment request
We need to determine the country based on the site locale, else it will always default to US
use-sf-payments.js — useSFPaymentsEnabled now checks both local config and API
Currently we only support the value in the API BUT this can break existing customers who might have already modified some of the customers. They can end up with a mix/match of with and without SF Payments. So, with this change, sf payments will always be enabled anyway. But customers have a kill switch to disable it if needed.
checkout/index.jsx — added missing await on doCreateOrder()
Handlebars (bootstrap and templates default.js.hbs) — need to change enabled: false to enabled: true for SF Payments
These handlebar changes are required so that customers who generate PWA based on our template will get the changes we added to SSR.js and default.js
Everything else (SLAS public/private, client IDs, _app-config changes) -- reverted back to public client instead of private client since PWA will be public by default in this release. SF Payments works in public client.
Note: There will be a more cleanup tasks in a subsequent PR
Types of Changes
Changes
How to Test-Drive This PR
Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization